Skip to content

[Cache] Custom pool #12904

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

[Cache] Custom pool #12904

wants to merge 1 commit into from

Conversation

Guikingone
Copy link
Contributor

Hi everyone 👋

It seems that when defining a custom pool, the documenation is talking about an array adapter when the used one is the filesystem, the question is, do we need to reword it as filesystem in the text of change the code? 🤔

Thanks for the feedback 🙂

@OskarStark
Copy link
Contributor

@Nyholm could you please give a review here? Thanks 🙏

Copy link
Member

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey.

Thank you for finding this mistake. I agree that it should be changed.
Maybe update the text instead of the config.

I also see that you introduce a cache chain here. That is not relevant for section.

cache.acme:
adapter: cache.adapter.memcached
cache.foobar:
adapter: cache.adapter.memcached
provider: 'memcached://user:password@example.com'

services:
app.my_cache_chain_adapter:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need to introduce a cache chain here. It i explained later.

@javiereguiluz javiereguiluz added this to the 3.4 milestone Mar 12, 2020
javiereguiluz added a commit that referenced this pull request Mar 12, 2020
This PR was merged into the 3.4 branch.

Discussion
----------

[Cache] Custom pool

Hi everyone 👋

It seems that when defining a custom pool, the documenation is talking about an array adapter when the used one is the filesystem, the question is, do we need to reword it as filesystem in the text of change the code? 🤔

Thanks for the feedback 🙂

Commits
-------

a50bce3 [FIX] Custom pool
@javiereguiluz
Copy link
Member

Guillaume, thanks for this ... but at the end we decided to follow Tobias suggestion and change instead the text description and keep the original code. Thanks!

@Guikingone
Copy link
Contributor Author

No problem @javiereguiluz, sorry for the delay, I was working on something else and I've forgot this PR, really sorry 🙁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants